tidb-operator: fix documentation usability issues in GCP document #519

yikeke · 2019-05-24T09:26:55Z

What problem does this PR solve?

fix documentation usability issues found in GCP deployment document

What is changed and how it works?

Check List

Tests

Unit test
E2E test
Stability test
Manual test (add detailed scripts or steps below)
No code

Code changes

Has Helm charts change
Has Go code change
Has CI related scripts change
Has documents change

Side effects

Breaking backward compatibility

Related changes

Need to cherry-pick to the release branch
Need to update the documentation

Does this PR introduce a user-facing change?:

CLAassistant · 2019-05-24T09:27:03Z

All committers have signed the CLA.

DanielZhangQD · 2019-05-24T12:31:53Z

deploy/gcp/README.md

 ```bash
 gcloud compute ssh bastion --zone <zone>
 mysql -h <tidb_ilb_ip> -P 4000 -u root
 ```

-It is possible to interact with the cluster using `kubectl` and `helm` with the kubeconfig file `credentials/kubeconfig_<cluster_name>`. The default `cluster_name` is `my-cluster`, it can be changed in `variables.tf`
+It is possible to interact with the cluster using `kubectl` and `helm` with the kubeconfig file `credentials/kubeconfig_<cluster_name>`. The default `cluster_name` is `my-cluster`, it can be changed in `variables.tf`:


Putting the descriptions for helm and kubectl in "access the database" section is not reasonable, suggest adding a new section like "interact with the cluster" or "manage the cluster"?

DanielZhangQD · 2019-05-24T12:33:20Z

deploy/gcp/README.md

+When you are done, the infrastructure can be torn down by running:
+
+``` shell


All "bash" in above codes, keep it same here？

DanielZhangQD · 2019-05-27T10:17:54Z

deploy/gcp/README.md

+
+> *Note*: The service account must have sufficient permissions to create resources in the project. The `Project Editor` primitive will accomplish this.
+
+To set the three environment variables, you can first run `vi ~/.bash_profile` and insert the `export` statements in it. Here is an example in `~/.bash_profile`:


Suggest to replace "insert" with "append", BTW, also need to source the file (source ~/.bash_profile or . ~/.bash_profile) after editing

DanielZhangQD · 2019-05-27T10:24:20Z

deploy/gcp/README.md


-> *Note*: Incrementing the node count will create a node per GCP availability zones.
+> *Note*: Incrementing the node count will create a node per GCP availability zone.


Should be "available zone"?

In the variables.tf file, the descriptions for tidb_count, tikv_count and other variables all use "per GCP availability zone", so I think we should unify the wording by using "availability" here.

yikeke · 2019-05-27T10:49:13Z

I have updated some examples and descriptions according to other deployment docs (see PR#507). PTAL @DanielZhangQD @jlerche @tennix

DanielZhangQD · 2019-05-27T11:29:01Z

deploy/gcp/README.md

+The upgrading does not finish immediately. You can run `kubectl --kubeconfig credentials/kubeconfig_<cluster_name> get po -n tidb --watch` to verify that all pods are in `Running` state. Then you can [access the database](#access-the-database) and use `tidb_version()` to see whether the cluster has been upgraded successfully:
+
+```sql
+MySQL [(none)]> select tidb_version()\G


Is "\G" required?

…-usability-issues-found-in-GCP-deployment-doc

jlerche · 2019-05-28T01:21:57Z

deploy/gcp/README.md

+> *Note*: The service account must have sufficient permissions to create resources in the project. The `Project Editor` primitive will accomplish this.
+
+To set the three environment variables, you can first run `vi ~/.bash_profile`, append the `export` statements to it and run `source ~/.bash_profile`. 
+


You can actually type export FOO=var or something like that in the terminal, so setting the environment variables in bash_profile is a little overkill. I would suggest removing the line "To set the three environment variables, you can first run vi ~/.bash_profile, append the export statements to it and run source ~/.bash_profile. "

If export in the terminal, after customer quit from the session and then open another session to run terraform, the variables need to be exported again, I think it's not convenient

jlerche · 2019-05-28T01:29:09Z

deploy/gcp/README.md

+
+To set the three environment variables, you can first run `vi ~/.bash_profile`, append the `export` statements to it and run `source ~/.bash_profile`. 
+
+Here is an example of `~/.bash_profile`:


Suggested change

Here is an example of `~/.bash_profile`:

For example, in your terminal you can enter:

Updated. PTAL again. @jlerche @DanielZhangQD

jlerche

LGTM

lilin90

LGTM

…n-GCP-deployment-doc

tidb-operator: fix documentation usability issues in GCP document

360adec

update wording

713afe8

DanielZhangQD reviewed May 24, 2019

View reviewed changes

yikeke added 8 commits May 27, 2019 11:31

address hailong's comments

bbd543c

update configure terraform

ad5e2d6

Update README.md

7fc8b82

add more descriptions for configure terraform

69f675f

update wording

1a9f650

update wording

8345618

add descriptions and examples

0ca9498

update wording

c7545b4

DanielZhangQD reviewed May 27, 2019

View reviewed changes

yikeke added 2 commits May 27, 2019 18:26

add some examples and update wording

6197efb

update examples and wording

b12270a

yikeke added 7 commits May 27, 2019 19:03

address hailong's comment

d8da9fc

improve display

c4868b4

update code block

02e9e27

unify wording

05ac1c3

update wording

c223730

update descriptions

eaf5853

Update README.md

7a90aa9

DanielZhangQD reviewed May 27, 2019

View reviewed changes

yikeke added 3 commits May 27, 2019 19:41

fix messy code

90d1732

Merge remote-tracking branch 'upstream/master' into Fix-documentation…

b54ae79

…-usability-issues-found-in-GCP-deployment-doc

fix messy code for dind tutorial

43adae1

jlerche suggested changes May 28, 2019

View reviewed changes

Update README.md

03717ef

yikeke added 2 commits May 28, 2019 10:59

address jacob's comments

ad02659

update wording

2a2a8a4

jlerche previously approved these changes May 28, 2019

View reviewed changes

update descriptions

6928911

yikeke dismissed jlerche’s stale review via 6928911 May 28, 2019 03:38

DanielZhangQD approved these changes May 28, 2019

View reviewed changes

lilin90 approved these changes May 28, 2019

View reviewed changes

Merge branch 'master' into Fix-documentation-usability-issues-found-i…

43cf493

…n-GCP-deployment-doc

tennix merged commit b0b71ca into pingcap:master May 28, 2019

yikeke deleted the Fix-documentation-usability-issues-found-in-GCP-deployment-doc branch May 28, 2019 05:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tidb-operator: fix documentation usability issues in GCP document #519

tidb-operator: fix documentation usability issues in GCP document #519

yikeke commented May 24, 2019 •

edited

Loading

CLAassistant commented May 24, 2019 •

edited

Loading

DanielZhangQD May 24, 2019 •

edited

Loading

DanielZhangQD May 24, 2019 •

edited

Loading

DanielZhangQD May 27, 2019

DanielZhangQD May 27, 2019

yikeke May 27, 2019

yikeke commented May 27, 2019 •

edited

Loading

DanielZhangQD May 27, 2019

jlerche May 28, 2019

DanielZhangQD May 28, 2019

jlerche May 28, 2019

yikeke May 28, 2019

jlerche left a comment

lilin90 left a comment

		When you are done, the infrastructure can be torn down by running:

		``` shell


		> Note: The service account must have sufficient permissions to create resources in the project. The `Project Editor` primitive will accomplish this.

		To set the three environment variables, you can first run `vi ~/.bash_profile` and insert the `export` statements in it. Here is an example in `~/.bash_profile`:


		> Note: Incrementing the node count will create a node per GCP availability zones.
		> Note: Incrementing the node count will create a node per GCP availability zone.


		To set the three environment variables, you can first run `vi ~/.bash_profile`, append the `export` statements to it and run `source ~/.bash_profile`.

		Here is an example of `~/.bash_profile`:

	Here is an example of `~/.bash_profile`:
	For example, in your terminal you can enter:

tidb-operator: fix documentation usability issues in GCP document #519

tidb-operator: fix documentation usability issues in GCP document #519

Conversation

yikeke commented May 24, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Does this PR introduce a user-facing change?:

CLAassistant commented May 24, 2019 • edited Loading

DanielZhangQD May 24, 2019 • edited Loading

Choose a reason for hiding this comment

DanielZhangQD May 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yikeke commented May 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlerche left a comment

Choose a reason for hiding this comment

lilin90 left a comment

Choose a reason for hiding this comment

yikeke commented May 24, 2019 •

edited

Loading

CLAassistant commented May 24, 2019 •

edited

Loading

DanielZhangQD May 24, 2019 •

edited

Loading

DanielZhangQD May 24, 2019 •

edited

Loading

yikeke commented May 27, 2019 •

edited

Loading